Skip to content

Conversation

@emilio
Copy link
Member

@emilio emilio commented Mar 9, 2017

Fixes #126

This change is Reviewable

@emilio
Copy link
Member Author

emilio commented Mar 9, 2017

r? @SimonSapin

@emilio
Copy link
Member Author

emilio commented Mar 9, 2017

Note that this is somewhat a wallpaper-ish patch, since the string would be lowercased if its length is less that the bindings.

We can disable bindings altogether otherwise I guess.

@SimonSapin
Copy link
Member

I think you’re right: we can’t have all three of lowercasing into a buffer on the stack to avoid allocation, support bindings in match arms, and get exactly the behavior of match &*input.to_ascii_lowercase() { /* … */ } (other than allocation). I’m also tempted to drop support for bindings in match arms.

@canaltinova What was your use case for bindings? Can you write your code without? Maybe replacing them like this:

-match_ignore_ascii_case! { &expect_keyword(),
+let keyword = expect_keyword();
+match_ignore_ascii_case! { &keyword,
     "foo" => Foo,
     "bar" => Bar,
-    keyword => Other(keyword.to_owned()),
+    _ => Other(keyword.to_owned()),
 }

@canova
Copy link
Contributor

canova commented Mar 10, 2017

will-change property accepts <custom-ident>. I was using this to convert to lowercase and exclude some unacceptable identifiers. Here's how I use this macro:

match_ignore_ascii_case! { &i.expect_ident()?,
    "will-change" | "none" | "all" | "auto" |
    "initial" | "inherit" | "unset" => Err(()),
    ident  => {
        Ok((Atom::from(ident)))
    }
}

Ah, <custom-ident> is actually case sensitive. I think I can live with that 😃

@SimonSapin
Copy link
Member

Ah, yes. If you want to get the lower-case version of an arbitrary string (of unbounded length), not just match it against a set of pre-defined patterns, then you should use s.to_ascii_lowercase() + match instead of match_ignore_ascii_case!.

@SimonSapin
Copy link
Member

Thanks for the PR @emilio but I think I’d rather remove support for match bindings. With this PR the bound value would sometimes be lower-cased and sometimes not, depending on its length. Closing, I’ll work on another PR now.

@SimonSapin SimonSapin closed this Apr 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants